-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable list of commands from playground repositories #897 - Commands Blacklisted #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yashbudhia thanks for contributing, please add tests for this.
pkg/util/blacklist.go
Outdated
"strings" | ||
) | ||
|
||
var blacklistedCommands = []string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we keep it as map bool, so that checks would be faster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be faster, let me apply the changes
pkg/util/blacklist.go
Outdated
} | ||
|
||
// IsBlacklistedCommand checks if a command is blacklisted | ||
func IsBlacklistedCommand(cmd string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename this to BlockListedCommand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
ok |
@yashbudhia please resolve conflicts once. |
I have resolved the conflicts and changed the name of the function to BlockListedCommand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the build
i have fixed the build and also removed the comments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
desired changes applied
Based on the issue Disable list of commands from playground repositories #897 in the main dice repository , this PR adds a
Tests -
Manually tested via postman for get as well as post routes and all blacklisted commands are throwing
errors in this fashion{"error": "ERR unknown command '{Command_name}"}
eg-
Scope for improvement
You can also test scenarios like: